Skip to content

Conversation

@renczesstefan
Copy link
Member

@renczesstefan renczesstefan commented Nov 5, 2025

Description

Fixes NAE-2245

Dependencies

No new dependencies were introduced

Third party dependencies

No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR.

How Has Been This Tested?

This was tested manually and with unit tests.

Test Configuration

Name Tested on
OS macOS Tahoe 26.0.1
Runtime Java 21
Dependency Manager Maven 3.9.9n
Framework version Spring Boot 3.4.4
Run parameters
Other configuration

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes have been checked, personally or remotely, with @...
  • I have commented my code, particularly in hard-to-understand areas
  • I have resolved all conflicts with the target branch of the PR
  • I have updated and synced my code with the target branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes:
    • Lint test
    • Unit tests
    • Integration tests
  • I have checked my contribution with code analysis tools:
  • I have made corresponding changes to the documentation:
    • Developer documentation
    • User Guides
    • Migration Guides

Summary by CodeRabbit

Bug Fixes

  • Corrected the data source used to retrieve role information, ensuring accurate identifiers and titles are displayed for non-global roles.

machacjozef and others added 2 commits October 23, 2025 10:20
- Updated parent POM version from `7.0.0-RC8.1` to `7.0.0-RC9` across all module POM files.
…tId field

Updated the code to use `role.processId` instead of `role.netId` when fetching the PetriNet object. This resolves potential mismatches and ensures accurate retrieval of process identifiers.
@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

A single method in ActionDelegate.groovy is updated to retrieve the PetriNet for non-global roles using role.processId instead of role.netId, affecting how the display identifier and title are determined for non-global roles in preference item collection.

Changes

Cohort / File(s) Summary
PetriNet retrieval for non-global roles
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Changed PetriNet lookup in collectRolesForPreferenceItem from petriNetService.get(new ObjectId(role.netId)) to petriNetService.get(new ObjectId(role.processId)) for non-global roles

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-line change with clear intent but requires verifying the semantic difference between role.netId and role.processId retrieval and confirming the PetriNet returned has the intended identifier and title values for display
  • Potential impact on role display labels in preference items should be validated through testing

Possibly related PRs

Suggested labels

bugfix

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly references the issue (NAE-2245) and describes the main fix: changing role preference menu item mapping from using a removed 'ne' (likely 'netId') to the correct identifier.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f11efc and e808e3f.

📒 Files selected for processing (1)
  • application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: machacjozef
Repo: netgrif/application-engine PR: 367
File: application-engine/src/main/resources/application.yaml:24-24
Timestamp: 2025-10-20T11:44:44.907Z
Learning: In the netgrif/application-engine project, the correction of the Elasticsearch task index name from "_taks" to "_task" in application.yaml was approved by maintainer machacjozef, indicating that any data migration concerns for this typo fix are handled separately or not applicable to their deployment scenario.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: Build
🔇 Additional comments (1)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1)

2424-2424: Fix is correct and design pattern is intentional.

The change from role.netId to role.processId is valid and properly used. PetriNetService provides two retrieval methods: get(ObjectId) at line 162 which retrieves a specific PetriNet version, and getNewestVersionByIdentifier(String) at line 288 which retrieves the newest version. The first overload intentionally gets the exact PetriNet version the role was created with (via ObjectId), while the second overload retrieves the latest version. This pattern is consistent throughout the codebase, including in ProcessRoleFactoryImpl.java.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added the bugfix A change that fixes a bug label Nov 5, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 5, 2025
@machacjozef machacjozef changed the base branch from release/7.0.0-rev9 to release/7.0.0-rev8 November 9, 2025 11:10
@machacjozef machacjozef dismissed coderabbitai[bot]’s stale review November 9, 2025 11:10

The base branch was changed.

# Conflicts:
#	application-engine/pom.xml
#	nae-object-library/pom.xml
#	nae-spring-core-adapter/pom.xml
#	nae-user-ce/pom.xml
#	nae-user-common/pom.xml
#	pom.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix A change that fixes a bug Extra Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants